The contents of this golden Verilog file seem incorrect for a 7x7 RAM. It appears to have been copied from an 8-bit wide RAM test and not fully updated.

Specifically:

• The module name is RAM7x8, but should be RAM7x7.
• The D and Q ports are 8 bits wide ([7:0]), but should be 7 bits ([6:0]).
• The memory array mem is declared as reg [7:0], but should be reg [6:0].
• The write logic uses an 8-bit part-select (i*8 +:8), but should use 7.

Please update this file to reflect the correct behavior for a 7-word by 7-bit RAM.

Gemini is correct here. Something not right with the behavioral model.

This large block of commented-out code should be removed. It appears to be legacy logic for resolving pin names that has been replaced by the new buildPinMap and *_pins_ maps. Keeping dead code can be confusing for future maintenance.

This info level log message inside a loop seems to be for debugging purposes. It can generate a lot of output and should be removed or changed to a more appropriate debug level before merging.

This loop for logging clock gate pins appears to be for debugging. It should be removed before merging.

The -read_ports argument is specified twice. This line is a duplicate and should be removed.

warning: no header providing "std::tie" is directly included [misc-include-cleaner]

src/ram/include/ram/ram.h:9:

- #include <utility>
+ #include <tuple>
+ #include <utility>

warning: no header providing "std::map" is directly included [misc-include-cleaner]

src/ram/include/ram/ram.h:7:

- #include <memory>
+ #include <map>
+ #include <memory>

warning: no header providing "std::map" is directly included [misc-include-cleaner]

src/ram/src/ram.cpp:10:

- #include <memory>
+ #include <map>
+ #include <memory>

warning: no header providing "std::tie" is directly included [misc-include-cleaner]

src/ram/include/ram/ram.h:10:

- #include <utility>
+ #include <tuple>
+ #include <utility>

I would change Pin --> Port in all cases as technically you are dealing with the logical port here.

Can you also update make_8x8 to make_8x8_sky130?

Should have some error checking to avoid using badly defined cells. e.g.

• Exactly 1 primary power
• Exactly 1 primary ground
• Exactly the number of pins which are expected for the given cell type

Or I guess I suppose you could check those things after the cell map has been made, if easier.

It doesn't need to be solved in this PR, but you should mark as a TODO item to check against various kinds of flip-flops to make sure invalid ones are flagged, e.g. scan flops, flip-flops with reset, etc. Anything that doesn't have only clk, D, and Q ports. There are occasionally some flip-flops that have a Qn (negated Q) output and those are ok, Qn is just left disconnected.

Gemini is correct here. Something not right with the behavioral model.

Don't need to include that here

This isn't technically sufficient. Although it would be really odd, it is still technically possible for other masters to have different power/ground names. A complete solution would be to iterate through all of the masters and collect the names into a std::set. Then, you would do a global connect on each member of the set.

99% of the time the set will only have 1 member, but it is good to handle all cases.

This is actually a separate item in #9392: "Add automatic detection of standard cell power/ground names"

It would be nice to keep that as a separate PR to only focus on one change.

Got it, I'll just keep the 'power_pin' and 'ground_pin' as required user-defined parameters now, and we can fix this in another PR.

I think ng45 has a tapcell, right? If not, use an x1 fill cell as the tap cell.

Does this work? I thought the metals needed to be the first 2 above the first metal layer.

We generally don't want to use above M3 if it can be helped.

I think Nangate45's density is just too high for us to route on metal2 and metal3. Any valid combination of pitch and width has resulted in violations with the detailed router. I will note that these pitch and width dimensions don't work when tapcells are inserted, so this test will probably change.

In c++20 you can use

auto operator<=>(const PortRole&) const = default;

and get all six operators

Why would a tristate enable appear as a DataIn? That doesn't appear possible in the prior loop.

The tristate enable pin has no distinguishing Liberty flag and is treated as an input port, so the only way to identify it is through the tristateEnable() expression on the tristate output port. Since port iteration order is arbitrary and PDK-dependent, the tristate output port may appear after the enable pin, meaning there is no guarantee the enable pin name is known before it is encountered during iteration. The other option is to do the check for the name first, before iterating over all the ports again and classifying them.

Got it. Perhaps a comment would help as it is a bit tricky

avoid manual memory management with a smart pointer and no delete

std::unique_ptr port_iter = liberty->portIterator();